NW | 26-SDC-Mar | TzeMing Ho | Sprint 2 | Chat App frontend and backend by TzeMingHo · Pull Request #78 · CodeYourFuture/Module-Decomposition

TzeMingHo · 2026-05-14T21:08:54Z

Learners, PR Template

Self checklist

I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
My changes meet the requirements of the task
I have tested my changes
My changes follow the style guide

Changelist

frontend link:
https://tzemingho-chatapp-server-frontend.hosting.codeyourfuture.io

backend link:
https://tzemingho-chatapp-server-backend.hosting.codeyourfuture.io

My prep link:
https://github.com/TzeMingHo/Project-Chat-App

cjyuan

The code could use some comments.

A useful rule of thumb is to ask yourself whether you could still explain how the code works six months from now. If the answer is uncertain, consider adding a comment to help future readers understand the intent and reasoning behind it.

Can you also include this item in the PR description?

A description of the additional features you have implemented.

cjyuan · 2026-05-24T09:28:40Z

+      <h1>Chat channel</h1>
+    </header>
+    <main>
+      <div id="chat-display-area"></div>


Currently the chat display area and the form can overlap when there are a lot of messages.

Can you make the necessary change or propose a method to overcome this issue?

cjyuan · 2026-05-24T09:30:41Z

+  // backendURL: "https://tzemingho-chatapp-server-backend.hosting.codeyourfuture.io",
+  backendURL: "http://localhost:4000",


This does not look like the deployed version. 😄

cjyuan · 2026-05-24T09:39:27Z

+  const messageInputElement = document.getElementById("message-input");
+  messageInputElement.addEventListener("input", (e) => {
+    state.messageString = e.target.value.trim();
+  });
+
+  const userInputElement = document.getElementById("user-name-input");
+  userInputElement.addEventListener("input", (e) => {
+    state.userString = e.target.value.trim();
+  });
+
+  document
+    .getElementById("message-submit-button")
+    .addEventListener("click", async (e) => {
+      await messageSubmitHandler(e, state.messageString, state.userString);
+    });


Why immediately update messageString and userString for every "input" event instead of extracting the input values only when the user clicks the submit button?

What do you think of this arrangement?

Assign messageSubmitHandler as the callback to the click event

Retrieve the two pieces of input in messageSubmitHandler

cjyuan · 2026-05-24T10:17:20Z

+    console.error(`Message or user cannot be empty.`);
+    window.alert("Message or user cannot be empty.");


Note: Neither of these approaches is a user-friendly mechanism for showing error messages.

No change needed, but should you need to further improve the app, this is an area to consider.

cjyuan · 2026-05-24T10:19:20Z

+  } catch (error) {
+    console.error(`Failed to post message: ${error}`);
+  }


Good job in anticipating potential errors (even if it is using only a placeholder error handling mechanism).

cjyuan · 2026-05-24T10:50:35Z

+    if (response.ok) {
+      const confirmMessage = await response.text();
+      if (confirmMessage == "sent") {
+        chatDisplay();


Why call chatDisplay() when state.messages remains unchanged at this point?

cjyuan · 2026-05-24T10:54:40Z

+const port = 4000;
+
+const waitingRoom = [];
+
+const chatHistory = [
+  {
+    message: "Welcome to the channel.",
+    user: "System",
+    timestamp: new Date().getTime(),
+  },
+];


Could consider moving these declarations before const app = express();

cjyuan · 2026-05-24T11:09:17Z

+  if (isNaN(since)) {
+    return res.json(chatHistory);
+  }
+  const newMessages = chatHistory.filter(({ timestamp }) => timestamp > since);


An approach to reduce the amount of data sent in the response. Nicely done!

cjyuan · 2026-05-24T12:22:35Z

+  const queryString = lastMessageTime ? `?since=${lastMessageTime}` : "";
+  const url = `${state.backendURL}/messages${queryString}`;
+  try {
+    const rawResponse = await fetch(url);


It is not obvious that this fetch() request may remain pending for up to 25 seconds without examining the server-side implementation.

Consider adding a comment to explain this behavior.

cjyuan · 2026-05-24T12:44:20Z

+  const callback = (message) => res.json([message]);
+  waitingRoom.push(callback);
+
+  const seconds = 25;
+  const milliseconds = 1000;
+
+  const timeout = setTimeout(() => {
+    const index = waitingRoom.indexOf(callback);
+    if (index !== -1) {
+      waitingRoom.splice(index, 1);
+      res.send([]);
+    }
+  }, seconds * milliseconds);


Took me quite a while to figure out how the "waiting room" logic works. I think it is over complicated.

These scenarios may be difficult to reproduce consistently, but there is a possibility that the "waiting room" logic could break under the following conditions:

HTTP responses arrive out of order due to the asynchronous nature of request processing.

Multiple users post messages concurrently, causing two or more messages to receive the same timestamp.

No change needed.

added files

474cc24

This comment has been minimized.

Sign in to view

TzeMingHo changed the title ~~added files~~ NW | May 14, 2026

This comment has been minimized.

Sign in to view

TzeMingHo added 📅 Sprint 2 Assigned during Sprint 2 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels May 14, 2026

This comment has been minimized.

Sign in to view

github-actions Bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label May 14, 2026

This comment has been minimized.

Sign in to view

chore: remove accidental node_modules and ignore them globally

da4170f

TzeMingHo added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label May 14, 2026

TzeMingHo added 2 commits May 22, 2026 15:40

switch back to localhost, and make timestamp at the backend

4c084cb

added a helper function to scroll to the bottom

1f6e2a7

cjyuan reviewed May 24, 2026

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels May 24, 2026

		// backendURL: "https://tzemingho-chatapp-server-backend.hosting.codeyourfuture.io",
		backendURL: "http://localhost:4000",

		console.error(`Message or user cannot be empty.`);
		window.alert("Message or user cannot be empty.");

Uh oh!

Conversation

TzeMingHo commented May 14, 2026

Learners, PR Template

Changelist

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

cjyuan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cjyuan left a comment •

edited

Loading